Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add description for Azure url format #1337

Merged
merged 3 commits into from
May 23, 2020
Merged

Add description for Azure url format #1337

merged 3 commits into from
May 23, 2020

Conversation

steffansluis
Copy link
Contributor

@steffansluis steffansluis commented May 22, 2020 •
edited
Loading

  • Add a table entry for the URL format to make use of dvc import-url with Microsoft Azure Blob Storage.
  • Add example for usage of Microsoft Azure Blob Storage with external dependencies.

You may disregard these recommendations if you used the Edit on GitHub button from dvc.org to improve a doc in place.

❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.

🐛 Please make sure to mention Fix #issue (if applicable) in the description of the PR. This causes GitHub to close it automatically when the PR is merged.

Please choose to allow us to edit your branch when creating the PR.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

@restyled-io restyled-io bot mentioned this pull request May 22, 2020
Closed
@steffansluis steffansluis mentioned this pull request May 22, 2020
Merged
3 tasks
@steffansluis steffansluis force-pushed the azure-external-dependencies branch from 4e67180 to 004617e Compare May 22, 2020 20:53
@steffansluis steffansluis force-pushed the azure-external-dependencies branch from 004617e to dae4297 Compare May 22, 2020 20:53
@efiop
Copy link
Contributor

efiop commented May 22, 2020

Thank you for the docs @steffansluis ! 🙏 Could you also add azure to https://dvc.org/doc/user-guide/external-dependencies , please?

@shcheklein
Copy link
Member

Thanks @steffansluis 🙏

@steffansluis steffansluis force-pushed the azure-external-dependencies branch from acbc7ed to a1af810 Compare May 22, 2020 23:06
@efiop efiop requested a review from jorgeorpinel May 22, 2020 23:14
jorgeorpinel
jorgeorpinel reviewed May 23, 2020
View reviewed changes
content/docs/user-guide/external-dependencies.md Outdated
Comment on lines 73 to 77
az storage copy \
-d data.json \
--source-account-name my-account \
--source-container my-container-name \
--source-blob data.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation here seems excessive? Code block lines shouldn't go over 72 characters long, please. How about just align the flags under copy? Or go further back if still they go over 72 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation is based on the indentation used in the HDFS example. I completely agree that it is excessive, but I figured there was probably a reason for it. I'll move it back a bit so they line up better with copy, I did check though and the longest line was 67 chars so it should be fine regardless

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing this to out attention @steffansluis I'll review this in a separate PR 🙂

jorgeorpinel
jorgeorpinel reviewed May 23, 2020
View reviewed changes
Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple minor comments so far. Thanks for the contribution!

steffansluis and others added 2 commits May 23, 2020 14:30
@steffansluis
Add example for Azure to external depedencies docs
08a9328
@steffansluis @jorgeorpinel
Update content/docs/command-reference/import-url.md
2ea62ad 
Apply unrelated suggested change: Rename `Google Storage` to `Google Cloud Storage`

Co-authored-by: Jorge Orpinel <[email protected]>
@steffansluis steffansluis force-pushed the azure-external-dependencies branch from d95546c to 2ea62ad Compare May 23, 2020 12:31
@steffansluis steffansluis requested a review from jorgeorpinel May 23, 2020 12:32
@shcheklein shcheklein temporarily deployed to dvc-landing-azure-exter-evfenr May 23, 2020 15:31 Inactive
@shcheklein shcheklein merged commit 47f8751 into iterative:master May 23, 2020
@shcheklein
Copy link
Member

Thanks @steffansluis 🙏

jorgeorpinel added a commit that referenced this pull request May 24, 2020
@jorgeorpinel
user-guide: review intentation of sample commands in external deps guide
fb498c2 
per #1337 (review)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shcheklein shcheklein shcheklein approved these changes

@jorgeorpinel jorgeorpinel Awaiting requested review from jorgeorpinel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@steffansluis @efiop @shcheklein @jorgeorpinel